Skip to content

bpo-45081: Fix __init__ method generation when inheriting from Protocol#28121

Merged
ambv merged 5 commits intopython:mainfrom
uriyyo:fix-issue-45081
Sep 2, 2021
Merged

bpo-45081: Fix __init__ method generation when inheriting from Protocol#28121
ambv merged 5 commits intopython:mainfrom
uriyyo:fix-issue-45081

Conversation

@uriyyo
Copy link
Member

@uriyyo uriyyo commented Sep 2, 2021 •
edited by bedevere-bot
Loading

@uriyyo uriyyo requested a review from ericvsmith as a code owner September 2, 2021 09:45
@uriyyo
Copy link
Member Author

uriyyo commented Sep 2, 2021

@Fidget-Spinner Could you please verify that we need backport those changes to 3.9 and 3.10 versions?

ericvsmith
ericvsmith reviewed Sep 2, 2021
View reviewed changes
Fidget-Spinner
Fidget-Spinner reviewed Sep 2, 2021
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uriyyo if I understand the original issue correctly, you're right that we need to backport this to fix that issue's backports.

I'm deferring this to @serhiy-storchaka or @ambv because I'm not too familiar with the previous patch that caused this regression. Sorry.

@uriyyo uriyyo requested a review from gvanrossum as a code owner September 2, 2021 12:03
@uriyyo
Copy link
Member Author

uriyyo commented Sep 2, 2021

@Fidget-Spinner Could you please review this PR? Looks like I found way how to fix this issue without changing dataclasses module.

Fidget-Spinner
Fidget-Spinner reviewed Sep 2, 2021
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After researching this issue (and the previous patch), this LGTM in general, I only have suggestions to change wording in some parts.

I'm not very confident with backporting this change so late into 3.10's release cycle. I hope another core dev can review.

Lib/typing.py
cls = type(self)

if cls._is_protocol:
raise TypeError('Protocols cannot be instantiated')
Copy link
Member

@Fidget-Spinner Fidget-Spinner Sep 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: this is the previous behavior.

uriyyo and others added 3 commits September 2, 2021 16:30
@uriyyo @Fidget-Spinner
Update Lib/test/test_dataclasses.py
6d1be74 
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@uriyyo
Update Ptocol init method
91b011e
@uriyyo
Update comment
8dc6a2e
@julianfortune julianfortune mentioned this pull request Sep 2, 2021
Closed
Fidget-Spinner
Fidget-Spinner approved these changes Sep 2, 2021
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I will wait a bit to see if Serhiy or Lukasz (or anyone else) says anything.

@Fidget-Spinner Fidget-Spinner changed the title bpo-45081: Fix dataclass __init__ method generation when inherit from Protocol bpo-45081: Fix __init__ method generation when inheriting from Protocol Sep 2, 2021
@ambv
Copy link
Contributor

ambv commented Sep 2, 2021

The previous behavior was introduced in GH-27545 as an attempt to make non-protocol subclasses of Protocol ignore the __init__ inherited from protocol base classes.

This PR retains this ability and restores compatibility with dataclasses. LGTM.

@ambv ambv merged commit 0635e20 into python:main Sep 2, 2021
@miss-islington
Copy link
Contributor

miss-islington commented Sep 2, 2021

Thanks @uriyyo for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 2, 2021
@uriyyo @miss-islington
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
cc82f4f 
…ol (pythonGH-28121)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)

Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 2, 2021
@bedevere-bot
Copy link

bedevere-bot commented Sep 2, 2021

GH-28131 is a backport of this pull request to the 3.10 branch.

@bedevere-bot
Copy link

bedevere-bot commented Sep 2, 2021

GH-28132 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 2, 2021
@uriyyo @miss-islington
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
de94c9e 
…ol (pythonGH-28121)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)

Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
ambv pushed a commit that referenced this pull request Sep 2, 2021
@miss-islington @uriyyo
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
98eb408 
…ol (GH-28121) (GH-28132)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)

Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@leahwicz leahwicz mentioned this pull request Sep 2, 2021
Closed
4 tasks
miss-islington added a commit that referenced this pull request Sep 3, 2021
@miss-islington @uriyyo
bpo-45081: Fix __init__ method generation when inheriting from Protoc…
79e9f5a 
…ol (GH-28121)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
(cherry picked from commit 0635e20)

Co-authored-by: Yurii Karabas <1998uriyyo@gmail.com>
@ronnie-llamado ronnie-llamado mentioned this pull request Sep 21, 2021
Closed
1 task
@JanEricNitschke JanEricNitschke mentioned this pull request Sep 11, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericvsmith ericvsmith ericvsmith left review comments

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@uriyyo @ambv @miss-islington @bedevere-bot @ericvsmith @Fidget-Spinner @the-knights-who-say-ni